MT-149497: net: fec: add imx8mm devinfo with UDP checksum quirk#42
MT-149497: net: fec: add imx8mm devinfo with UDP checksum quirk#42Overdr0ne wants to merge 6 commits into
Conversation
The imx8mm FEC hardware RX checksum offload engine incorrectly marks certain UDP packets as checksum-valid, causing them to be silently dropped by the kernel UDP stack. Add a dedicated fsl,imx8mm-fec compatible string, which previously fell through to fsl,imx8mq-fec, and a new FEC_QUIRK_ERR_UDP_CSUM quirk. When the quirk is set, downgrade CHECKSUM_UNNECESSARY to CHECKSUM_NONE for UDP packets only after the HW reports no error, forcing the kernel UDP stack to re-validate the checksum in software. TCP checksum offload is unaffected.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds i.MX8MM FEC support and introduces a quirk/workaround for a hardware RX checksum offload issue affecting UDP.
Changes:
- Add i.MX8MM-specific
fec_devinfoand register it via platform IDs + DT compatible match. - Introduce a new quirk bit (
FEC_QUIRK_ERR_UDP_CSUM) for the i.MX8MM UDP RX checksum validation issue. - Adjust RX checksum handling to force software validation for UDP when the quirk is enabled.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| drivers/net/ethernet/freescale/fec_main.c | Registers imx8mm variant and changes RX checksum behavior under a new quirk. |
| drivers/net/ethernet/freescale/fec.h | Defines a new quirk flag to gate the UDP checksum workaround. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
devQA: Failed Due to urgency of this fix, tested devQA prior to code review - currently fails. @Overdr0ne lets connect Monday AM and I can show you how to reproduce the issue on your local machine between DVSC and MT Connect. Note that this involves using the linklocal addressing fix which is in current origin/develop branch. |
The implementation only handles IPv4/UDP, but the comment described the quirk as applying to UDP generally. Update the comment to match.
skb_network_header() never returns NULL, making the null check ineffective. It also does not handle non-linear skbs, risking out-of-bounds reads on malformed or non-IPv4 frames. Replace with skb_header_pointer() into a local buffer, and guard the block with an ETH_P_IP protocol check before treating the header as an IPv4 header.
|
@AmeNote-Michael I've resolved the review comments. I'm seeing a lot of mixed information about this issue. It seemed like you were not seeing the oscillations even on a build that did have rx-checksumming enabled. Then Ben said Audinate had made some sort of patch on their end that fixed some things? What's the current status here? |
The i.MX8MM FEC hardware incorrectly identifies certain valid UDP frames as having bad protocol checksums when PRODIS (protocol discard) is active in the RACC register, causing them to be silently dropped before the driver ever receives them. Based on a patch distributed by Audinate with their i.MX8MM EVK. Scoped to imx8mm via the existing FEC_QUIRK_ERR_UDP_CSUM flag rather than changing FEC_RACC_OPTIONS globally, so all other FEC variants retain their existing behaviour.
val is read from the FEC_RACC register before being modified. When the FEC_QUIRK_ERR_UDP_CSUM quirk masks PRODIS out of racc_opts, the subsequent 'val |= racc_opts' could not clear a PRODIS bit that was already set in the register, leaving protocol discard active. Clear all FEC_RACC_OPTIONS bits in val first, then OR in the desired options.
The quirk now drives two distinct workarounds: clearing FEC_RACC_PRODIS in fec_restart() and downgrading CHECKSUM_UNNECESSARY in the RX path. The comment only mentioned the latter; document both.
MT-149497
So I basically just had Claude implement the Option B fix here. I've only really done a build test so far.
The imx8mm FEC hardware RX checksum offload engine incorrectly marks certain UDP packets as checksum-valid, causing them to be silently dropped by the kernel UDP stack.
Add a dedicated fsl,imx8mm-fec compatible string, which previously fell through to fsl,imx8mq-fec, and a new FEC_QUIRK_ERR_UDP_CSUM quirk. When the quirk is set, downgrade CHECKSUM_UNNECESSARY to CHECKSUM_NONE for UDP packets only after the HW reports no error, forcing the kernel UDP stack to re-validate the checksum in software. TCP checksum offload is unaffected.
dev QA: @AmeNote-Michael